fix(orchestrator): bound the whole sweep, not one more call inside it - #374

… tree

A sweep went silent for 11m53s and then reported stalled (inFlightMs 727833,
missedPasses 12) while the process stayed healthy — liveHeartbeat logging
throughout, log ring lossless, consecutiveFailures 0. Nothing was failing. The
sweep was slow by construction, in `resolveIssuePrFromMount`, which answers
"which PR belongs to this issue" by reading every mounted PR record one at a
time.

Four defects, all in the same walk.

1. `#resolveIssuePr` could not scope to a repository. `resolveIssuePrFromMount`
   has always honoured `opts.repo`, but `#resolveIssuePr`'s own opts had no
   `repo` field, so it forwarded `undefined` and walked EVERY configured
   repository — 21 in the live workspace — to find a PR that can only live in
   one. None of its four call sites nor the `probePrResolver` port could scope
   it. Adds `repo?: string` and threads the routing answer through all of them
   via `#probeRepoForIssue`, which reuses `dependencyRepoForIssue` — the same
   helper `#dependencyIsTerminalOrMerged` already uses for its own probe.
   Ambiguous routing still walks unscoped: narrowing on a guess would miss a PR
   that is really there.

2. The mount hit never populated the cache it reads. `#resolveIssuePr` reads
   `#probePrResolvedCache` at the top, then runs the mount walk FIRST — the
   common hit — and returned without ever writing it. The cache had a reader
   and no writer on the hot path, so the whole walk repeated per caller, per
   sweep, forever. Now cached on the same terms as the gh branch: same key,
   same TTL, same draft exclusion.

3. The walk read most pull requests twice. `githubPullRoots` returns two roots
   for one repository — the nested `<owner>/<repo>/pulls/` layout and the flat
   `<owner>__<repo>/pulls/by-id/` alias — and unions them into a Set keyed by
   PATH STRING, so one PR under two spellings counted twice. Deduped on the
   identity the path already carries via `githubPullPathParts`, which costs no
   read. Paths that carry no PR identity (`_index.json`, per-PR `comments/*`)
   are left in the walk and still read exactly as before.

4. The read loop was invisible. `listTree` is wrapped by `#listRelayfileTree` —
   named, timed, logged. The `readFile` per candidate ran in a bare try/catch
   that swallows failures into `undefined`, with no logger, counter or progress
   line, which is why twelve minutes of real work was indistinguishable from a
   hung process for three prior investigation layers. Adds progress reporting on
   the same cadence helper the ready-issue read loop uses, plus a
   `probePrMountReads` counter.

Also, two things found while fixing the above:

- The cache invalidation on completion deleted only the BARE issue key, never
  the `:open` / `:legacy` suffixed variants `#resolveIssuePr` actually writes.
  Every `openOnly` probe — i.e. the completion path — was never invalidated.
  Harmless while the mount branch wrote nothing; a live correctness bug the
  moment it does. Now clears the whole key family.

- `#dependencyIsTerminalOrMerged` does not go through `#resolveIssuePr` (it must
  not fall back to gh), so it saw no cache at all, and
  `#terminalDependencyIdentities` memoises only the TRUE answer. A dependency
  that is not merged was re-walked in full for every issue declaring it, on
  every sweep. Adds a sweep-scoped memo for the negative answer, cleared beside
  the terminal set so a PR merging between sweeps is still observed. This is the
  path that produced the reported repro.

Relationship to #374, which bounds the whole sweep: complementary, not
redundant. #374 stops a wedge burning unbounded wall-clock — the seatbelt. This
removes the reason the walk is expensive — the brakes. relayfile-adapters#271
would remove the walk entirely by putting `headRef` in the pull index row.

NOT FIXED, deliberately: no early break on a maximal-score match. The sort is
`b.score - a.score || b.prNumber - a.prNumber`, so a score-30 hit does not win
until every higher-numbered candidate is known to score no better, and
`readProbePrCandidate` takes `pr.number` from the payload rather than the path,
so path order does not prove PR-number order. Semantics could not be shown
preserved, so per the brief the dedupe ships and the early break does not.

No index fast path either: `pulls/_index.json` rows carry no `headRef`, and the
primary match (score 30) is a branch match, so the index cannot exclude any PR
from consideration and a title hit (score 20) must never be returned while an
unread branch match could outrank it. Instead the resolver now logs WHY it fell
back — index absent, shape unrecognised, or present without `headRef` — so the
day adapters#271 lands shows up in the logs rather than passing unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

… tree (#377)

* fix(orchestrator): stop the probe PR resolver re-walking the whole PR tree

A sweep went silent for 11m53s and then reported stalled (inFlightMs 727833,
missedPasses 12) while the process stayed healthy — liveHeartbeat logging
throughout, log ring lossless, consecutiveFailures 0. Nothing was failing. The
sweep was slow by construction, in `resolveIssuePrFromMount`, which answers
"which PR belongs to this issue" by reading every mounted PR record one at a
time.

Four defects, all in the same walk.

1. `#resolveIssuePr` could not scope to a repository. `resolveIssuePrFromMount`
   has always honoured `opts.repo`, but `#resolveIssuePr`'s own opts had no
   `repo` field, so it forwarded `undefined` and walked EVERY configured
   repository — 21 in the live workspace — to find a PR that can only live in
   one. None of its four call sites nor the `probePrResolver` port could scope
   it. Adds `repo?: string` and threads the routing answer through all of them
   via `#probeRepoForIssue`, which reuses `dependencyRepoForIssue` — the same
   helper `#dependencyIsTerminalOrMerged` already uses for its own probe.
   Ambiguous routing still walks unscoped: narrowing on a guess would miss a PR
   that is really there.

2. The mount hit never populated the cache it reads. `#resolveIssuePr` reads
   `#probePrResolvedCache` at the top, then runs the mount walk FIRST — the
   common hit — and returned without ever writing it. The cache had a reader
   and no writer on the hot path, so the whole walk repeated per caller, per
   sweep, forever. Now cached on the same terms as the gh branch: same key,
   same TTL, same draft exclusion.

3. The walk read most pull requests twice. `githubPullRoots` returns two roots
   for one repository — the nested `<owner>/<repo>/pulls/` layout and the flat
   `<owner>__<repo>/pulls/by-id/` alias — and unions them into a Set keyed by
   PATH STRING, so one PR under two spellings counted twice. Deduped on the
   identity the path already carries via `githubPullPathParts`, which costs no
   read. Paths that carry no PR identity (`_index.json`, per-PR `comments/*`)
   are left in the walk and still read exactly as before.

4. The read loop was invisible. `listTree` is wrapped by `#listRelayfileTree` —
   named, timed, logged. The `readFile` per candidate ran in a bare try/catch
   that swallows failures into `undefined`, with no logger, counter or progress
   line, which is why twelve minutes of real work was indistinguishable from a
   hung process for three prior investigation layers. Adds progress reporting on
   the same cadence helper the ready-issue read loop uses, plus a
   `probePrMountReads` counter.

Also, two things found while fixing the above:

- The cache invalidation on completion deleted only the BARE issue key, never
  the `:open` / `:legacy` suffixed variants `#resolveIssuePr` actually writes.
  Every `openOnly` probe — i.e. the completion path — was never invalidated.
  Harmless while the mount branch wrote nothing; a live correctness bug the
  moment it does. Now clears the whole key family.

- `#dependencyIsTerminalOrMerged` does not go through `#resolveIssuePr` (it must
  not fall back to gh), so it saw no cache at all, and
  `#terminalDependencyIdentities` memoises only the TRUE answer. A dependency
  that is not merged was re-walked in full for every issue declaring it, on
  every sweep. Adds a sweep-scoped memo for the negative answer, cleared beside
  the terminal set so a PR merging between sweeps is still observed. This is the
  path that produced the reported repro.

Relationship to #374, which bounds the whole sweep: complementary, not
redundant. #374 stops a wedge burning unbounded wall-clock — the seatbelt. This
removes the reason the walk is expensive — the brakes. relayfile-adapters#271
would remove the walk entirely by putting `headRef` in the pull index row.

NOT FIXED, deliberately: no early break on a maximal-score match. The sort is
`b.score - a.score || b.prNumber - a.prNumber`, so a score-30 hit does not win
until every higher-numbered candidate is known to score no better, and
`readProbePrCandidate` takes `pr.number` from the payload rather than the path,
so path order does not prove PR-number order. Semantics could not be shown
preserved, so per the brief the dedupe ships and the early break does not.

No index fast path either: `pulls/_index.json` rows carry no `headRef`, and the
primary match (score 30) is a branch match, so the index cannot exclude any PR
from consideration and a title hit (score 20) must never be returned while an
unread branch match could outrank it. Instead the resolver now logs WHY it fell
back — index absent, shape unrecognised, or present without `headRef` — so the
day adapters#271 lands shows up in the logs rather than passing unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

* fix(orchestrator): never scope a probe to repos.default, and key the probe cache by repo

Two #377 review findings from cubic-dev-ai. Both were correct; the first was a
correctness regression this PR introduced.

P1 — `#probeRepoForIssue` scoped every probe to `repos.default` whenever the
issue carried no label or project evidence. Routing precedence is byLabel,
byProject, keywordRules, default, and `dependencyRepoForIssue` can see neither
the triage decision nor `keywordRules` — those match on issue TEXT through
triage. So for a keyword-routed issue it answered `repos.default` while dispatch
had opened the PR in the keyword-selected repository. The probe then walked one
repository, confidently, and found nothing: "no PR" reported for an issue that
has one, and the completion path acts on that answer. That is strictly worse
than the slow walk this PR set out to remove, and it contradicted the docstring
sitting two lines above it.

Threading the real triage decision was not reachable: all five probe call sites
take only a `LinearIssue`, and at completion time the decision no longer exists.
So the fallback is now the unscoped walk — `dependencyRepoForIssue` grows an
opt-out `allowDefault` (default unchanged for its four other callers) and the
probe wrapper passes `false`. Ambiguity widens the walk; it never narrows it.
The dedupe and cache in this same PR already blunt the cost.

P2 — `repo` narrows which pull requests a resolution can even see, so it is a
resolution dimension, but it was absent from the cache and gh-backoff keys. A
route change could therefore serve the previous repository's PR, and the
completion path probes and CLOSES what it is handed.

Adding that dimension exposed a second, pre-existing defect: the completion
sweep wrote its draft-PR backoff under a BARE issue state key while
`#completionPrForIssue` read the suffixed one. They agreed only by accident, and
the new suffix broke that accident — caught by `gh PR fallback skips draft PRs
and backs off repeated unresolved lookups`, which went red. Both maps now build
their key through one shared `#probePrCacheKey`, so the two writers cannot
drift again. Every dimension stays a trailing `:`-prefixed segment, so the
completion invalidation added in this PR keeps clearing the whole key family.

NOT SHIPPED: a test for P2's stale cross-repo hit. Probe scope is a pure
function of (issue, config) at all five call sites, and the completion path
clears the whole key family, so no public path varies the scope for one issue
inside the TTL. Every way to force it needed a production test hook, and this
file has no precedent for reaching into internals. P2 ships as defensive
correctness plus the real backoff-key fix its test DID catch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

…e cadence that cannot

THIS COMMIT DOES NOT ADD A BOUND. It publishes the ones that already exist,
because their absence from the health stanza has now been read twice as their
absence from the code — including in the brief that asked for this fix.

WHAT THE STANZA SAID. A wedged 0.1.76 published:

    "readinessReconcile": {
      "state": "healthy", "consecutiveFailures": 0,
      "failureThreshold": 3, "inFlightMs": 268232, "intervalMs": 60000
    }

`intervalMs` is a scheduler tick and cannot preempt anything. Next to an
`inFlightMs` climbing 1:1 with wall clock it is indistinguishable from an
unbounded hang, and there was no field that could tell the two apart. The
reading taken from it — "there is no timeoutMs, so nothing bounds this" — is
false, and it is the reading this stanza invites.

WHAT IS ACTUALLY BOUNDING THAT PASS. Three deadlines, all live on this path in
0.1.76: `relayfileOperationTimeoutMs` per call (#351/#368),
`readinessReconcileTimeoutMs` on the caller's wait (#296), and the aggregate
`sweepBudgetMs` from #374 — `#reconcileReadyIssues` -> `#runOnceWithReadinessDeadline`
-> `runOnce()` -> `#runOnceWithDiscoveryFence` -> `startDiscoverySweepBudget`.
`readinessReconcile` IS the discovery sweep's health stanza; sweep-budget.ts
names it as such. The pass was bounded. It was bounded at 90 minutes, because
`sweepBudgetMs` derives from `reconcileTimeoutMs`, so at 268 s it had 89
minutes left to run and no field said so.

Two numbers now ship: `timeoutMs` (ends the wait) and `sweepBudgetMs` (unwinds
the sweep and hands the lease back). The second is the one that answers "when
does this recover", which is the question every reader of this stanza has
actually been asking.

`missedPasses` also moves onto the heartbeat record. It already existed on the
public projection (#295/#300) and was absent from the heartbeat stanza — which
is the surface an operator opens first, and the one every report so far has
quoted.

NOT A REPORTING BUG, AND DELIBERATELY NOT CHANGED. `state: "healthy"` at
268 s is correct. `derivedReadinessReconcileState` re-derives `stalled` from
`inFlightMs > intervalMs * READINESS_RECONCILE_STALL_INTERVALS`, and that
constant is 10 — so the flip was due at 600 s and the observation window
(14:37Z-14:41Z) closed 5.5 minutes early. Lowering it is the trivially wrong
fix: public-health.ts documents #36's 61-minute post-boot hydration as the
reason a small multiple cries wolf on every cold container.

TESTS, both against the real production numbers:
- must-fire: a heartbeat carrying the bounds publishes both, and reports
  missedPasses 4 for the exact 268232/60000 pass above. Fail-first verified by
  ablation — with only public-health.ts and types.ts reverted it fails
  `expected undefined to be 5400000`.
- must-not-fire: a recorded `0` or negative bound is dropped rather than
  republished as an instant deadline, and an instance predating the fields
  still projects `healthy` with both absent. This one passes before and after
  by construction: it is the guard on the trivially wrong version, not a
  demonstration of the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

…e sweep bounds that already exist (#379)

* fix(health): publish the bounds that can preempt a sweep, not just the cadence that cannot

THIS COMMIT DOES NOT ADD A BOUND. It publishes the ones that already exist,
because their absence from the health stanza has now been read twice as their
absence from the code — including in the brief that asked for this fix.

WHAT THE STANZA SAID. A wedged 0.1.76 published:

    "readinessReconcile": {
      "state": "healthy", "consecutiveFailures": 0,
      "failureThreshold": 3, "inFlightMs": 268232, "intervalMs": 60000
    }

`intervalMs` is a scheduler tick and cannot preempt anything. Next to an
`inFlightMs` climbing 1:1 with wall clock it is indistinguishable from an
unbounded hang, and there was no field that could tell the two apart. The
reading taken from it — "there is no timeoutMs, so nothing bounds this" — is
false, and it is the reading this stanza invites.

WHAT IS ACTUALLY BOUNDING THAT PASS. Three deadlines, all live on this path in
0.1.76: `relayfileOperationTimeoutMs` per call (#351/#368),
`readinessReconcileTimeoutMs` on the caller's wait (#296), and the aggregate
`sweepBudgetMs` from #374 — `#reconcileReadyIssues` -> `#runOnceWithReadinessDeadline`
-> `runOnce()` -> `#runOnceWithDiscoveryFence` -> `startDiscoverySweepBudget`.
`readinessReconcile` IS the discovery sweep's health stanza; sweep-budget.ts
names it as such. The pass was bounded. It was bounded at 90 minutes, because
`sweepBudgetMs` derives from `reconcileTimeoutMs`, so at 268 s it had 89
minutes left to run and no field said so.

Two numbers now ship: `timeoutMs` (ends the wait) and `sweepBudgetMs` (unwinds
the sweep and hands the lease back). The second is the one that answers "when
does this recover", which is the question every reader of this stanza has
actually been asking.

`missedPasses` also moves onto the heartbeat record. It already existed on the
public projection (#295/#300) and was absent from the heartbeat stanza — which
is the surface an operator opens first, and the one every report so far has
quoted.

NOT A REPORTING BUG, AND DELIBERATELY NOT CHANGED. `state: "healthy"` at
268 s is correct. `derivedReadinessReconcileState` re-derives `stalled` from
`inFlightMs > intervalMs * READINESS_RECONCILE_STALL_INTERVALS`, and that
constant is 10 — so the flip was due at 600 s and the observation window
(14:37Z-14:41Z) closed 5.5 minutes early. Lowering it is the trivially wrong
fix: public-health.ts documents #36's 61-minute post-boot hydration as the
reason a small multiple cries wolf on every cold container.

TESTS, both against the real production numbers:
- must-fire: a heartbeat carrying the bounds publishes both, and reports
  missedPasses 4 for the exact 268232/60000 pass above. Fail-first verified by
  ablation — with only public-health.ts and types.ts reverted it fails
  `expected undefined to be 5400000`.
- must-not-fire: a recorded `0` or negative bound is dropped rather than
  republished as an instant deadline, and an instance predating the fields
  still projects `healthy` with both absent. This one passes before and after
  by construction: it is the guard on the trivially wrong version, not a
  demonstration of the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

* fix(orchestrator): bound the completion release retry, which is what was actually spinning

The `no pid available to terminate ... during completion` lines repeating 15+
times in one evidence payload are a CO-SYMPTOM, not the cause. Fixing the PID
classification would have changed nothing, and this commit explains why before
it changes anything.

WHY THE MISSING PID IS NOT THE LOOP. `#releaseAndTerminateAgents` logs that
line when `#terminationRoots` returns `{ pids: [], status: 'unresolved' }`,
then falls through. Nothing on that branch reaches `failed[]` — only a throw
from `#fleet.release()` that is not `isAgentAlreadyGoneOnRelease` does. So the
three agents were re-attempted because their RELEASE kept failing, and the
no-PID line was printed once per agent per attempt on the way past.

WHY THE LOOP NEVER ENDED. `#finishDurableRelease` does not throw on a failed
release: it returns `false` and calls `#scheduleReleaseRetry`, which re-arms at
`DISPATCH_LIFECYCLE_RETRY_MS` — 1 000 ms, unbounded. Every re-arm therefore
arrives on the RESOLVED path, which is why the `.catch()` in both schedulers
never bounded it and why a bound written there would have been a fix that never
fired. The budget is charged at the scheduling point instead.

WHAT A PASS COSTS, WHICH IS WHY 1 Hz FOREVER IS NOT FREE. Each pass calls
`#terminationRoots` once per agent inside the release AND once per agent again
inside `#writeInFlightRegistry` — a process-table scan each — plus a durable
lifecycle read and write. For the three agents in the report that is order ten
scans and several state operations per second, indefinitely. #303 already
measured this exact shape once, at 1477 state GETs in 111 s, and bounded the
RATE of the capacity-wait re-arm in response. It deliberately left the COUNT
unbounded there, because waiting for capacity is legitimate.

DESIGN CHOICE: (a) BOUNDED RETRIES, NOT (b) RECLASSIFY NO-PID. Not chosen under
uncertainty — the code already tells the two cases apart, and it says (b) is
wrong. `#terminationRoots` returns `'missing'` for confirmed-gone (a remote
placement, or a process scan that came back missing AND a resolver that agreed)
and `'unresolved'` for could-not-determine (no resolver and no recorded pids,
an AMBIGUOUS scan, a resolver that returned nothing, or one that threw). The
error only fires on `'unresolved'`. Treating that as already-terminated would
mean skipping termination of a process that may well be alive — an ambiguous
scan is literally "more than one candidate matched" — leaving orphans holding
worktrees and slots. And it would not have stopped the spin regardless, per the
first section.

Release is also the opposite shape from #303's capacity wait, which is what
makes bounding the count right here and wrong there: it is the last step of a
work unit that is already finished — issue closed, writeback acknowledged,
batch slot returned — so a release that has failed ten times is not waiting for
anything. Ten attempts at the 1 s floor is ~10 s of genuine retry, which covers
a control-plane blip or a lease handover and does not cover a permanent
failure.

SCOPED SO IT CANNOT ABANDON WORK THAT WAS NEVER FAILING:
- Only release re-arms spend the budget. `#scheduleDispatchLifecycleRetry`
  takes an explicit `releaseAttempt` flag, so a `DispatchLifecycleCapacityError`
  or `DispatchLifecycleOwnedElsewhereError` — both legitimate waits on someone
  else — still retries forever, exactly as #303 intended.
- Progress refunds the budget, so ten bounds CONSECUTIVE no-progress passes
  rather than capping a slow multi-agent release. This terminates: an agent
  released once is checkpointed and skipped next pass, so the remaining set
  strictly shrinks and a refund can only be earned finitely often.
- The durable lifecycle is RETAINED on exhaustion. A takeover or a restart
  re-drives it from the persisted phase. This bounds one process's spin; it
  does not declare the work unit clean.
- Keyed by `dispatchLifecycleKey`, so the budget follows the work unit rather
  than an agent, a surface or a dispatcher — the AR-448 identity rule.

Exhaustion is logged at `error`, not `warn`, and increments
`dispatchLifecycleReleaseAbandoned`. Every layer of this failure so far has
been invisible until somebody read stderr by hand, and a work unit whose
cleanup this process has permanently given up on must not be inferable only
from the absence of further log lines.

TESTS (3), against a fleet that reproduces the production shape exactly —
`release()` throws for `issue-done` and `resolveAgentPid` returns
`'unresolved'`, so the same no-PID line is emitted on every pass:
- must-fire: the dead-letter counter reaches 1, the exhaustion error is logged,
  and three further seconds of wall clock buy no additional release attempts.
  Fail-first verified by ablation: with factory.ts reverted it fails after
  40 543 ms with `expected undefined to be 1` — the wait can only end in its
  own deadline, because the loop re-arms for as long as the process lives.
  That is a property of the loop, not of any number chosen in the test.
- must-not-fire: a release that succeeds still completes the work unit and
  releases each agent exactly once, with the counter unset. The trivially wrong
  way to stop a retry loop is to stop retrying.
- must-not-fire: a release that fails several times and then succeeds still
  completes, with the counter unset — the transient case the retry exists for.
  Both must-not-fires passed under the ablation too, which is what makes them
  guards rather than restatements of the fix.

RETRY CADENCE IS NOW AN INJECTABLE PORT, and that is a test-stability fix in
its own right rather than a convenience. Exhausting a ten-attempt budget at the
real 1 s floor costs ten real seconds per case; the first version of this suite
did exactly that and added 41 s to `factory.test.ts`. Run beside two other
files it pushed an already-300 s combination over an edge and four UNRELATED
tests began failing on timing — the reopen-fence and Slack-reply-route cases —
while the same three files passed on `origin/main` and `factory.test.ts` alone
passed 631/631 on the branch. Buying a fourth flake in this suite (it already
carries #342 and #373) to test a fix for a spin is the wrong trade.

`dispatchLifecycleRetryMs` follows the existing convention for exactly this —
`babysitterWakeUnreachableRetryMs`, `babysitterWakeUnreachableEscalateMs`,
`startupAgentExitDrainTimeoutMs` are all test-only port overrides of a built-in
timing. Only the delay between attempts moves; the BUDGET under test is the
real one. Overhead is now +4 s, the four unrelated failures are gone (709/709
on the same three files), and the ablation still fails with `expected undefined
to be 1` — unambiguously, because `dispatchLifecycleReleaseAbandoned` does not
exist on `origin/main` at any cadence.

The transient case sets a failure count on the fake rather than flipping a flag
from the test body, so it cannot race the cadence it runs under.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7

* fix(orchestrator): make the release bound actually fire on the durable path

Answers three P1 findings on #379. The first is the important one: the bound
as first written DID NOT FIRE in production, and the review caught it.

1. THE BUDGET RESET ON THE PATH THAT MATTERS, SO THE BOUND NEVER FIRED.

   `#driveDispatchLifecycle` discards `#finishDurableRelease`'s boolean in its
   `phase === 'releasing'` branch, and that method returns `false` rather than
   throwing on a failed release. So a FAILED release makes the drive RESOLVE,
   and the scheduler's success handler ran on every re-arm — where it called
   `#clearReleaseAttempts`. The counter was zeroed once per pass and could
   never reach the cap.

   This is the same never-fires shape the first version of this commit
   correctly rejected in the `.catch()`, moved one layer over into the
   `.then()`. Diagnosing the resolved path as the live one and then putting the
   refund on it was the error.

   The refund is removed from the scheduler entirely. It now happens only where
   success is actually known: `#finishDurableRelease` clears the budget on real
   per-agent progress and again when the work unit completes.

   WHY THE ORIGINAL TESTS MISSED IT. `#usesDurableDispatchLifecycle()` is
   `durableOwnership ?? placementLocality === 'remote'`, and `FakeFleetClient`
   places locally, so all three original cases exercised `#scheduleReleaseRetry`'s
   own timer — which has no success handler and therefore no reset. The
   deployed Factory places remotely. The suite proved a property of the path
   production does not take.

   New must-fire on the DURABLE path (`RemoteLifecycleFleetClient` +
   `InMemoryStateStore`), asserting the counter SURVIVES ACROSS RE-ARMS rather
   than that a dead-letter is reachable by some path. Fail-first verified by
   ablation: restore the `#clearReleaseAttempts(key)` line and only that case
   fails, `expected undefined to be 1` after 10 125 ms, while the three local
   cases still pass — which is what pins the discrimination to the durable path.

2. THE WRONG BUDGET WAS CHARGED.

   The generic arm of the drive's `.catch()` re-arms for dispatch, publishing
   and recovery failures as well as releases, and it charged all of them. That
   would dead-letter a work unit that was never stuck in a release loop.

   Charging is now confined to `#scheduleReleaseRetry`, whose every caller is a
   release failure: the three inside `#finishDurableRelease`, and
   `#completeIssue`'s catch once `releaseReasonForRetry` is set. The generic
   re-arm passes no charge at all.

   Pinned by a call-site audit rather than by a behavioural test, and that is
   deliberate. I could not reach that arm from a realistic fixture — forcing
   durable lifecycle reads to throw makes the agent-exit handler fail before any
   lifecycle retry is scheduled, so a test built that way passes whether or not
   the narrowing is present. Confirmed by ablation: with `releaseAttempt = true`
   restored, the fixture-based version still passed, and instrumenting it showed
   zero `durable dispatch lifecycle retry failed` warnings — the branch was
   never entered. Shipping that would have been a test that proves nothing, so
   the audit states the structure instead.

   Known gap, stated plainly: a release failure that THREW out of
   `#finishDurableRelease` would reach the generic arm and re-arm unbounded.
   Every failure path in that method returns `false` and schedules its own
   retry, so this is not a reachable shape today, and if one appears it degrades
   to the pre-existing unbounded behaviour rather than to a wrong dead-letter.

3. THE DEAD-LETTER LEAKED THE SLOT.

   Trading an unbounded 1 Hz spin for a permanently leaked in-flight record is
   not obviously the better failure: a spin is loud and self-describing, while a
   leaked slot silently reduces dispatch capacity until the process is
   restarted. Local completion never calls `batch.complete`, so exhaustion left
   the work unit in flight forever.

   `#releaseDeadLetteredSlot` now hands the batch slot back, drops any
   uncompensated claim, rewrites the in-flight registry, and admits whatever was
   queued behind it — a freed slot nothing is admitted into is only half the
   repair. The durable lifecycle is still deliberately RETAINED in `releasing`,
   so a successor or restart re-drives the same cleanup with a fresh budget;
   freeing a process-local slot is not a terminal phase and does not declare the
   work clean. The work unit therefore ends up recoverable, never merely
   abandoned.

   Must-fire asserts the slot is released after exhaustion. Fail-first by
   ablation: stub the call out and it fails with the work unit still in flight.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Session-Id: 1a0d070b-c811-486f-9a86-9e9743becdb7